Skip to content

Make MarkRevealed and RecordWonBid order-independent - #170

Open
damilolaedwards wants to merge 1 commit into
ethpandaops:mainfrom
damilolaedwards:fix-payment-tracker-reveal-ordering
Open

Make MarkRevealed and RecordWonBid order-independent#170
damilolaedwards wants to merge 1 commit into
ethpandaops:mainfrom
damilolaedwards:fix-payment-tracker-reveal-ordering

Conversation

@damilolaedwards

Copy link
Copy Markdown
Contributor

Problem

MarkRevealed (fired by RevealService's own gate/timer) and RecordWonBid
(fired by InclusionTracker's head-event loop) are driven by independent
goroutines with no happens-before edge between them. Whenever a head event
is delayed past the reveal gate, MarkRevealed can run first: it found no
pending entry, silently skipped the balance deduction, and the RecordWonBid
that arrived afterward created a pending entry that was never marked
revealed - orphaned until it expired two epochs later, understating the
effective balance for the whole window.

Fix

PaymentTracker now records a slot in a small earlyReveals set when
MarkRevealed can't find a pending entry, instead of just giving up. When
RecordWonBid later runs for that slot, it applies the deduction immediately
and skips creating a pending entry, since the bid was already revealed.
Stale early-reveal markers (a reveal recorded but the matching won bid
report never arrives) are cleared by the same two-epoch prune pass that
already clears expired pending payments.

Testing

TestPaymentTracker_MarkRevealedBeforeRecordWonBid reproduces the exact
race and asserts the deduction now lands correctly with nothing left
pending; TestPaymentTracker_RecordWonBidBeforeMarkRevealed confirms the
common ordering is unaffected; TestPaymentTracker_EarlyRevealNeverRecordedIsPruned
confirms the leak-prevention path.

Verified the main regression test actually catches the bug: reverted just
the production file, ran the test, and it failed with exactly the predicted
numbers (balance adjustment stuck at 0 instead of -5000, 5000 stuck in
pending instead of 0), then restored the fix and confirmed it passes.

go build, go vet, and go test -race ./pkg/... all pass (aside from a
pre-existing, unrelated failure in pkg/webui caused by the frontend not
being built in this checkout).

MarkRevealed (fired by RevealService's own gate/timer) and RecordWonBid
(fired by InclusionTracker's head-event loop) are driven by independent
goroutines with no happens-before edge between them. Whenever a head event
is delayed past the reveal gate, MarkRevealed can run first: it found no
pending entry, silently skipped the balance deduction, and the
RecordWonBid that arrived afterward created a pending entry that was never
marked revealed - orphaned until it expired two epochs later, understating
the effective balance for the whole window.

PaymentTracker now records a slot in a small earlyReveals set when
MarkRevealed can't find a pending entry, instead of just giving up. When
RecordWonBid later runs for that slot, it applies the deduction immediately
and skips creating a pending entry, since the bid was already revealed.
Stale early-reveal markers (a reveal recorded but the matching won bid
report never arrives) are cleared by the same two-epoch prune pass that
already clears expired pending payments.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant